fix: require Nordic DFU service UUID to select a DFU target#7
Open
balloob wants to merge 1 commit into
Open
Conversation
find_dfu_target selected the first device matching MAC+1 OR a loose signal that included a name merely containing "DFU". Nordic bootloaders all advertise as "DfuTarg", so with two tags in bootloader mode (or any third-party DfuTarg nearby) the name-only path could select and flash the wrong device. Keep MAC+1 as the primary discriminator and, for the address-changed fallback, require the Nordic Legacy DFU service UUID (00001530-...) in the advertisement — a specific signal Legacy bootloaders always send. Drop name-only selection. Also document that the no-carry MAC+1 (uint8 addr[0] increment, wrap at 0xFF) is the intended Nordic-reference convention and that the py-opendisplay companion aligns to it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
find_dfu_targetselected the first scanned device matching the expected MAC+1 address OR a loose DFU signal that included a name merely containing"DFU". Nordic bootloaders all advertise under the generic nameDfuTarg, so when two tags are in bootloader mode at once (or any unrelated third-partyDfuTargis nearby), the name-only path could select — and then flash — the wrong device.Fix
Keep the deterministic MAC+1 address as the primary discriminator. For the fallback that covers bootloaders which don't retain the MAC+1 address, require the Nordic Legacy DFU service UUID (
00001530-...) to be present in the advertisement — a specific signal that Legacy bootloaders always advertise. Name-only selection is dropped, so a device that merely hasDFUin its name (without the service UUID or a matching address) is no longer picked.A unit test now asserts a name-only
DfuTargadvertisement (no service UUID, non-matching address) is not selected, alongside the existing tests for MAC+1 and service-UUID matches.MAC+1 convention note
The no-carry MAC+1 here (a
uint8increment ofaddr[0], wrapping at0xFFwith no carry into the next byte) is the documented Nordic-reference convention and is intentionally left as-is; a comment now records this. The py-opendisplay companion aligns its MAC+1 logic to match this repo's no-carry behaviour.🤖 Generated with Claude Code
https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
Companion PR: OpenDisplay/py-opendisplay#121 (py-opendisplay side — aligns MAC+1 to this repo's no-carry convention + UUID-based discovery fallback, no name-based selection). Consistent discriminator across both.